We should not rely on the status until the sts controller has observed the new generation.

or:

• use actualStatefulSets.PendingReconciliation() before that loop
• create a common function to reuse the logic in

  cloud-on-k8s/pkg/controller/elasticsearch/driver/expectations.go

  Lines 23 to 46 in cb0d001

  	func (d *defaultDriver) expectationsSatisfied(ctx context.Context) (bool, string, error) {
  	log := ulog.FromContext(ctx)
  	// make sure the cache is up-to-date
  	expectationsOK, reason, err := d.Expectations.Satisfied()
  	if err != nil {
  	return false, "", err
  	}
  	if !expectationsOK {
  	log.V(1).Info("Cache expectations are not satisfied yet, re-queueing", "namespace", d.ES.Namespace, "es_name", d.ES.Name, "reason", reason)
  	return false, reason, nil
  	}
  	actualStatefulSets, err := sset.RetrieveActualStatefulSets(d.Client, k8s.ExtractNamespacedName(&d.ES))
  	if err != nil {
  	return false, "", err
  	}
  	// make sure StatefulSet statuses have been reconciled by the StatefulSet controller
  	pendingStatefulSetReconciliation := actualStatefulSets.PendingReconciliation()
  	if len(pendingStatefulSetReconciliation) > 0 {
  	log.V(1).Info("StatefulSets observedGeneration is not reconciled yet, re-queueing", "namespace", d.ES.Namespace, "es_name", d.ES.Name)
  	return false, fmt.Sprintf("observedGeneration is not reconciled yet for StatefulSets %s", strings.Join(pendingStatefulSetReconciliation.Names().AsSlice(), ",")), nil
  	}
  	// make sure pods have been reconciled by the StatefulSet controller
  	return actualStatefulSets.PodReconciliationDone(ctx, d.Client)
  	}

I think we should use UpdateReplicas(...) instead of updating directly Replicas, to also update common.k8s.elastic.co/template-hash

Should we not use es_sset.ReconcileStatefulSet() instead of calling k8sClient.Update(..) directly? It already includes the call to expectations.ExpectGeneration(reconciled).

And maybe a follow-up question is should we update UpscaleResults with the result of es_sset.ReconcileStatefulSet() to work with a consistent view in the rest of the driver logic.

I was trying to understand in which cases we can get a 404 IIUC one of them is when the user is attempting to scale up the masters with a new nodeset? Let's maybe add a godoc to explain that we are only scaling up existing master statfulsets, new ones are ignored.

I just realized that calling this when len(nonMasterResources) == 0 (or more generally, when all non-master nodesets have already been upgraded?) can be slightly suboptimal.

Assuming that the initial state is:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 9.1.0
  nodeSets:
  - name: default
    config:
      node.roles: ["master", "data", "ingest", "ml"]
      node.store.allow_mmap: false
    count: 3

If we update and upgrade to:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 9.1.2
  nodeSets:
  - name: default
    config:
      node.roles: ["master", "data", "ingest", "ml"]
      node.store.allow_mmap: false
    count: 4

Then we are going to scale up the 9.1.0 statefulset, leading to the creation of elasticsearch-sample-es-default-3, but immediately in the next reconciliation we are going to delete elasticsearch-sample-es-default-3 to upgrade it to 9.1.2

My previous comment made me wonder if !isVersionUpgrade is actually the only reason we might want to reconcile everything at once.

Will this test fail if we consistently get an error here? (my feeling is that it's not going to be the case because violations is always empty in that case, but maybe I'm missing something)

Without WithMutatedFrom the data integrity index has no replicas, which means that the cluster is going to be red as soon as one Pod is recreated, and the test is going to fail.

Edit: confirmed by the e2e test I started in my previous buildkite comment

{
    "Time": "2025-12-03T09:12:11.122554149Z",
    "Action": "output",
    "Package": "github.com/elastic/cloud-on-k8s/v3/test/e2e/es",
    "Test": "TestNonMasterFirstUpgradeComplexTopology/Elasticsearch_cluster_health_should_not_have_been_red_during_mutation_process",
    "Output": "{\"log.level\":\"error\",\"@timestamp\":\"2025-12-03T09:12:11.117Z\",\"message\":\"continuing with additional tests\",\"service.version\":\"0.0.0-SNAPSHOT+00000000\",\"service.type\":\"eck\",\"ecs.version\":\"1.4.0\",\"error\":\"test Elasticsearch cluster health should not have been red during mutation process failed\",\"error.stack_trace\":\"github.com/elastic/cloud-on-k8s/v3/test/e2e/test.StepList.RunSequential\n\t/go/src/github.com/elastic/cloud-on-k8s/test/e2e/test/step.go:52\ngithub.com/elastic/cloud-on-k8s/v3/test/e2e/test.RunMutationsWhileWatching\n\t/go/src/github.com/elastic/cloud-on-k8s/test/e2e/test/run_mutation.go:77\ngithub.com/elastic/cloud-on-k8s/v3/test/e2e/es.runNonMasterFirstUpgradeTest\n\t/go/src/github.com/elastic/cloud-on-k8s/test/e2e/es/non_master_first_upgrade_test.go:76\ngithub.com/elastic/cloud-on-k8s/v3/test/e2e/es.TestNonMasterFirstUpgradeComplexTopology\n\t/go/src/github.com/elastic/cloud-on-k8s/test/e2e/es/non_master_first_upgrade_test.go:102\ntesting.tRunner\n\t/usr/local/go/src/testing/testing.go:1792\"}\n"
}